Skip to content

Implement logout redirection for reverse proxy auth setups#36085

Merged
wxiaoguang merged 3 commits intogo-gitea:mainfrom
eliroca:logout_redirect_main
Apr 10, 2026
Merged

Implement logout redirection for reverse proxy auth setups#36085
wxiaoguang merged 3 commits intogo-gitea:mainfrom
eliroca:logout_redirect_main

Conversation

@eliroca
Copy link
Copy Markdown
Contributor

@eliroca eliroca commented Dec 4, 2025

When authentication is handled externally by a reverse proxy SSO provider, users can be redirected to an external logout URL or relative path defined on the reverse proxy.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 4, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files docs-update-needed The document needs to be updated synchronously labels Dec 4, 2025
@eliroca eliroca force-pushed the logout_redirect_main branch 4 times, most recently from 63a3a81 to fa7ee5a Compare December 9, 2025 09:33
@wxiaoguang
Copy link
Copy Markdown
Contributor

After Implements OIDC RP-Initiated Logout #36724, you can redirect to anywhere you want, and no need to play with the frontend JS anymore.

@silverwind
Copy link
Copy Markdown
Member

So we close this?

@wxiaoguang
Copy link
Copy Markdown
Contributor

So we close this?

Why?

@eliroca
Copy link
Copy Markdown
Contributor Author

eliroca commented Mar 3, 2026

So we close this?

I'll rebase and update the patch this week.

@silverwind
Copy link
Copy Markdown
Member

So we close this?

Why?

Sorry, misread.

@eliroca eliroca force-pushed the logout_redirect_main branch from fa7ee5a to 29ae0fb Compare April 3, 2026 16:53
@github-actions github-actions bot removed the modifies/templates This PR modifies the template files label Apr 3, 2026
@eliroca eliroca changed the title Enable logout redirection for reverse proxy setups Implement logout redirection for reverse proxy setups Apr 3, 2026
@eliroca eliroca force-pushed the logout_redirect_main branch from 29ae0fb to 6be3cad Compare April 3, 2026 17:04
@eliroca
Copy link
Copy Markdown
Contributor Author

eliroca commented Apr 8, 2026

Can this be included in the upcoming 1.26.0 release?

lunny
lunny previously approved these changes Apr 8, 2026
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 8, 2026
silverwind
silverwind previously approved these changes Apr 8, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 8, 2026
@eliroca
Copy link
Copy Markdown
Contributor Author

eliroca commented Apr 9, 2026

Don't force-push to PR branches please.

Why not? Github is able to handle it and show only what changed between states.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 9, 2026

I can't see what has changed since your last commit. It's all just one commit now. It's important for the review to see how feedback was addressed.

We will squash on merge, so there's no reason to prematurely squash on the PR branch.

@eliroca
Copy link
Copy Markdown
Contributor Author

eliroca commented Apr 9, 2026

I can't see what has changed since your last commit. It's all just one commit now. It's important for the review to see how feedback was addressed.

We will squash on merge, so there's no reason to prematurely squash on the PR branch.

There is a way to see what changed since last commit, but I get your point, I'll try to keep the force-pushing to a minimum.

image

@silverwind
Copy link
Copy Markdown
Member

Interesting, I didn't know GitHub had that feature. But yes prefer not to force-push still 😉.

@wxiaoguang
Copy link
Copy Markdown
Contributor

I can't see what has changed since your last commit. It's all just one commit now. It's important for the review to see how feedback was addressed.
We will squash on merge, so there's no reason to prematurely squash on the PR branch.

There is a way to see what changed since last commit, but I get your point, I'll try to keep the force-pushing to a minimum.

No, it only shows the diff between last force push and previous force push, the more history commits are still lost.

https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md

image

@wxiaoguang
Copy link
Copy Markdown
Contributor

#36085 (comment)

And, if a user is logged in via non-reverse-proxy-auth, should they also be redirected to the ReverseProxyLogoutRedirect ?

What do you think about this case? For example: on an instance, reverse-proxy-auth is enabled, and a user login via other methods (e.g.: OAuth2 or password form), is it possible?

@eliroca
Copy link
Copy Markdown
Contributor Author

eliroca commented Apr 10, 2026

What do you think about this case? For example: on an instance, reverse-proxy-auth is enabled, and a user login via other methods (e.g.: OAuth2 or password form), is it possible?

I think if reverse-proxy-auth is enabled, no other authentication methods should be.

@wxiaoguang
Copy link
Copy Markdown
Contributor

What do you think about this case? For example: on an instance, reverse-proxy-auth is enabled, and a user login via other methods (e.g.: OAuth2 or password form), is it possible?

I think if reverse-proxy-auth is enabled, no other authentication methods should be.

OK, let's write this assumption into comments.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 10, 2026
@wxiaoguang wxiaoguang requested review from lunny and silverwind April 10, 2026 10:04
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 10, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 10, 2026 12:35
@wxiaoguang wxiaoguang disabled auto-merge April 10, 2026 12:35
@wxiaoguang wxiaoguang changed the title Implement logout redirection for reverse proxy setups Implement logout redirection for reverse proxy auth setups Apr 10, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 10, 2026 12:36
@wxiaoguang wxiaoguang merged commit 16d7817 into go-gitea:main Apr 10, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 10, 2026
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Apr 10, 2026
…36085)

When authentication is handled externally by a reverse proxy SSO
provider, users can be redirected to an external logout URL or relative
path defined on the reverse proxy.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Apr 10, 2026
@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind removed the docs-update-needed The document needs to be updated synchronously label Apr 10, 2026
silverwind pushed a commit that referenced this pull request Apr 10, 2026
…37171)

Backport #36085 by @eliroca

When authentication is handled externally by a reverse proxy SSO
provider, users can be redirected to an external logout URL or relative
path defined on the reverse proxy.

Co-authored-by: Elisei Roca <eroca@suse.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/done All backports for this PR have been created backport/v1.26 This PR should be backported to Gitea 1.26 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants